Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update conformance for EGs #287

Merged
merged 8 commits into from
Dec 23, 2024
Merged

Update conformance for EGs #287

merged 8 commits into from
Dec 23, 2024

Conversation

SteveLLamb
Copy link
Member

Change default text of the Conformance section for EG docs, and check for conformance language within the doc (outside of the default sections.

@SteveLLamb SteveLLamb requested a review from palemieux October 10, 2024 00:57
Copy link

smpte.js Outdated
sections.forEach((element) => {
let id = element.id;
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) {
if (element.innerText.includes("shall")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to handle uppercase and lowercase.

... and what about equivalent forms, e.g., "need to be"? See Clause 7 at https://www.iso.org/sites/directives/current/part2/index.xhtml

Copy link
Member Author

@SteveLLamb SteveLLamb Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to handle uppercase and lowercase.

Addressed in c17ea95

... and what about equivalent forms, e.g., "need to be"? See Clause 7 at https://www.iso.org/sites/directives/current/part2/index.xhtml

I'm using this text from the Conformance clause we use:

Normative text is text that describes elements of the design that are indispensable or contains the 
conformance language keywords: "shall", "should", or "may". Informative text is text that is potentially 
helpful to the user, but not indispensable, and can be removed, changed, or added editorially without 
affecting interoperability. Informative text does not contain any conformance keywords

I would argue that the keywords are what are usually used, and need to be flagged. Past that it's an editorial thing, and I cannot recall ever seeing any of those equivalents in a SMPTE doc TBH. Also, do EGs docs NEED to conform to ISO guidelines for equivalent forms? I'm just using the OM as a baseline as a first pass.

@SteveLLamb SteveLLamb requested a review from palemieux October 10, 2024 16:21
doc/main.html Outdated Show resolved Hide resolved
smpte.js Outdated Show resolved Hide resolved
smpte.js Show resolved Hide resolved
smpte.js Outdated Show resolved Hide resolved
smpte.js Outdated Show resolved Hide resolved
smpte.js Outdated
if (docMetadata.pubType === smpte.EG_PUBTYPE) {

const sections = document.querySelectorAll('section');
sections.forEach((element) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use for...of when the body of the loop is huge.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tired this the first time around, but the element links don't pass through via the logger_.error messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should work:

const list = document.querySelectorAll("input[type=checkbox]");
for (const checkbox of list) {
  checkbox.checked = true;
}

https://developer.mozilla.org/en-US/docs/Web/API/NodeList#example

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is updated in fc85e00

smpte.js Outdated
const sections = document.querySelectorAll('section');
sections.forEach((element) => {
let id = element.id;
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((id !== "sec-front-matter") && (id !== "sec-foreword") && (id !== "sec-conformance")) {
if (id === "sec-front-matter" || id === "sec-foreword" && id === "sec-conformance")
continue;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic i have catches the sections that are to be excluded from the searching. This update only highlights those sections, and doesn't continue the search past those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is right. I think the following code style:

while () {

  if (bad) continue;

  Do a lot of things;
}

is easier to read and maintain than:

while () {

  if (not bad) {
    Do a lot of things;
  }
}

because Do a lot of things; can be big and you have to remember you are in a particular condition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yea understood. I meant the or and and combination. I did the same thing you mention with return; in the last update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in fc85e00

@SteveLLamb SteveLLamb requested a review from palemieux October 16, 2024 17:02
smpte.js Show resolved Hide resolved
@SteveLLamb SteveLLamb merged commit 5c34c08 into main Dec 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants